Skip to content

Conversation

dconeybe
Copy link
Contributor

This is a follow-up to #885 and fixes the Snappy build on some older compilers (e.g. gcc-9.4.0 on linux).

This PR is based on this patch: google/snappy#128

The build error looks like this:

snappy.cc:1017:8: warning: always_inline function might not be inlinable [-Wattributes]
 1017 | size_t AdvanceToNextTag(const uint8_t** ip_p, size_t* tag) {
      |        ^~~~~~~~~~~~~~~~
snappy.cc: In function ‘std::pair<const unsigned char*, long int> snappy::DecompressBranchless(const uint8_t*, const uint8_t*, ptrdiff_t, T, ptrdiff_t) [with T = char*]’:
snappy.cc:1017:8: error: inlining failed in call to always_inline ‘size_t snappy::AdvanceToNextTag(const uint8_t**, size_t*)’: function body can be overwritten at link time
snappy.cc:1097:43: note: called from here
 1097 |         size_t tag_type = AdvanceToNextTag(&ip, &tag);
      |                           ~~~~~~~~~~~~~~~~^~~~~~~~~~~

The fix is to explicitly mark AdvanceToNextTag() as inline.

e.g. https://github.com/firebase/firebase-unity-sdk/runs/5998971917

@dconeybe dconeybe added the skip-release-notes Skip release notes check label Apr 13, 2022
@dconeybe dconeybe self-assigned this Apr 13, 2022
@dconeybe dconeybe added api: firestore tests-requested: quick Trigger a quick set of integration tests. labels Apr 13, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. labels Apr 13, 2022
@github-actions
Copy link

github-actions bot commented Apr 13, 2022

❌  Integration test FAILED

Requested by @dconeybe on commit fd054fa
Last updated: Wed Apr 13 11:21 PDT 2022
View integration test log & download artifacts

Failures Configs
database [TEST] [FLAKINESS] [Android] [1/3 os: macos] [android_target]
(1 failed tests)  FirebaseDatabaseTest.TestInfoConnected
firestore [TEST] [FAILURE] [iOS] [macos] [1/2 ios_device: simulator_target]
(1 failed tests)  NumericTransformsTest.CreateDocumentWithIncrement

Add flaky tests to go/fpl-cpp-flake-tracker

cynthiajoan
cynthiajoan previously approved these changes Apr 13, 2022
@github-actions github-actions bot dismissed cynthiajoan’s stale review April 13, 2022 02:09

🍞 Dismissed stale approval on external PR.

@dconeybe dconeybe added tests-requested: quick Trigger a quick set of integration tests. and removed tests: in-progress This PR's integration tests are in progress. labels Apr 13, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. labels Apr 13, 2022
@github-actions github-actions bot added the tests: failed This PR's integration tests failed. label Apr 13, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Apr 13, 2022
@github-actions github-actions bot added the tests: succeeded This PR's integration tests succeeded. label Apr 13, 2022
@dconeybe dconeybe removed the tests: failed This PR's integration tests failed. label Apr 13, 2022
@dconeybe dconeybe merged commit fd054fa into main Apr 13, 2022
@dconeybe dconeybe deleted the dconeybe/SnappyLinuxGccFixBuild branch April 13, 2022 14:39
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests: succeeded This PR's integration tests succeeded. labels Apr 13, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Apr 13, 2022
@firebase firebase locked and limited conversation to collaborators May 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: firestore skip-release-notes Skip release notes check tests: failed This PR's integration tests failed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants